Fix for integer overflow in BinaryHeap for ZSTs #149451
Conversation
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use |
This comment has been minimized.
This comment has been minimized.
| // If hole has only one child. | ||
| // | ||
| // child <= end - 1 | ||
| // => 2 * hole + 1 <= end - 1 | ||
| // => hole <= (end - 2) / 2 | ||
| // => hole < 1 + (end - 2) / 2 | ||
| // => hole < end / 2 | ||
| if hole.pos() < end / 2 { | ||
| let child = 2 * hole.pos() + 1; | ||
| // If we are not in order, move parent | ||
| if hole.element() < unsafe { hole.get(child) } { | ||
| unsafe { hole.move_to(child) }; | ||
| } | ||
| } |
There was a problem hiding this comment.
You shouldn't just delete the safety comments.
There was a problem hiding this comment.
Although they also don't need to be complex; just need to point out why hole.pos() != child and both are valid indices.
There was a problem hiding this comment.
Oh sorry. I feel the new code is different than what we had before so the short circuit safety comment (before the if statement) can be removed? I will add the c = 2 * h + 1 safety comment back
There was a problem hiding this comment.
Yeah, the point the original was making was just that cond && unsafe { can_assume_here_that(cond == true) }, which was kind of tangential to the actual unsafe.
5 participants
Fixes #149448
This PR fixes a potential integer overflow while calculating the children of a node for a BinaryHeap for zero sized types.